[GlobalOpt] Revert global widening transform #144652
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Partially reverts e37d736. The transform has a number of correctness and code quality issues, and will benefit from a from-scratch re-review more than incremental fixes.
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesPartially reverts e37d736. The transform has a number of correctness and code quality issues, and will benefit from a from-scratch re-review more than incremental fixes. The correctness issues are hinted at in #144641, but I think it needs a larger rework to stop working ArrayTypes and the implementation could use some other improvements (like callInstIsMemcpy should just be Patch is 29.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144652.diff 14 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 4a06e0fa619c0..583db604905cd 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -92,8 +92,6 @@ STATISTIC(NumInternalFunc, "Number of internal functions");
STATISTIC(NumColdCC, "Number of functions marked coldcc");
STATISTIC(NumIFuncsResolved, "Number of statically resolved IFuncs");
STATISTIC(NumIFuncsDeleted, "Number of IFuncs removed");
-STATISTIC(NumGlobalArraysPadded,
- "Number of global arrays padded to alignment boundary");
static cl::opt<bool>
OptimizeNonFMVCallers("optimize-non-fmv-callers",
@@ -2037,165 +2035,6 @@ OptimizeFunctions(Module &M,
return Changed;
}
-static bool callInstIsMemcpy(CallInst *CI) {
- if (!CI)
- return false;
-
- Function *F = CI->getCalledFunction();
- if (!F || !F->isIntrinsic() || F->getIntrinsicID() != Intrinsic::memcpy)
- return false;
-
- return true;
-}
-
-static bool destArrayCanBeWidened(CallInst *CI) {
- auto *IsVolatile = dyn_cast<ConstantInt>(CI->getArgOperand(3));
- auto *Alloca = dyn_cast<AllocaInst>(CI->getArgOperand(0));
-
- if (!Alloca || !IsVolatile || IsVolatile->isOne())
- return false;
-
- if (!Alloca->isStaticAlloca())
- return false;
-
- if (!Alloca->getAllocatedType()->isArrayTy())
- return false;
-
- return true;
-}
-
-static GlobalVariable *widenGlobalVariable(GlobalVariable *OldVar,
- unsigned NumBytesToPad,
- unsigned NumBytesToCopy) {
- if (!OldVar->hasInitializer())
- return nullptr;
-
- ConstantDataArray *DataArray =
- dyn_cast<ConstantDataArray>(OldVar->getInitializer());
- if (!DataArray)
- return nullptr;
-
- // Update to be word aligned (memcpy(...,X,...))
- // create replacement with padded null bytes.
- StringRef Data = DataArray->getRawDataValues();
- std::vector<uint8_t> StrData(Data.begin(), Data.end());
- for (unsigned int p = 0; p < NumBytesToPad; p++)
- StrData.push_back('\0');
- auto Arr = ArrayRef(StrData.data(), NumBytesToCopy + NumBytesToPad);
- // Create new padded version of global variable.
- Constant *SourceReplace = ConstantDataArray::get(OldVar->getContext(), Arr);
- GlobalVariable *NewGV = new GlobalVariable(
- *(OldVar->getParent()), SourceReplace->getType(), true,
- OldVar->getLinkage(), SourceReplace, SourceReplace->getName());
- // Copy any other attributes from original global variable
- // e.g. unamed_addr
- NewGV->copyAttributesFrom(OldVar);
- NewGV->takeName(OldVar);
- return NewGV;
-}
-
-static void widenDestArray(CallInst *CI, const unsigned NumBytesToPad,
- const unsigned NumBytesToCopy,
- ConstantDataArray *SourceDataArray) {
-
- auto *Alloca = dyn_cast<AllocaInst>(CI->getArgOperand(0));
- if (Alloca) {
- unsigned ElementByteWidth = SourceDataArray->getElementByteSize();
- unsigned int TotalBytes = NumBytesToCopy + NumBytesToPad;
- unsigned NumElementsToCopy = divideCeil(TotalBytes, ElementByteWidth);
- // Update destination array to be word aligned (memcpy(X,...,...))
- IRBuilder<> BuildAlloca(Alloca);
- AllocaInst *NewAlloca = BuildAlloca.CreateAlloca(ArrayType::get(
- Alloca->getAllocatedType()->getArrayElementType(), NumElementsToCopy));
- NewAlloca->takeName(Alloca);
- NewAlloca->setAlignment(Alloca->getAlign());
- Alloca->replaceAllUsesWith(NewAlloca);
- Alloca->eraseFromParent();
- }
-}
-
-static bool tryWidenGlobalArrayAndDests(GlobalVariable *SourceVar,
- const unsigned NumBytesToPad,
- const unsigned NumBytesToCopy,
- ConstantInt *BytesToCopyOp,
- ConstantDataArray *SourceDataArray) {
- auto *NewSourceGV =
- widenGlobalVariable(SourceVar, NumBytesToPad, NumBytesToCopy);
- if (!NewSourceGV)
- return false;
-
- // Update arguments of remaining uses that
- // are memcpys.
- for (auto *User : SourceVar->users()) {
- auto *CI = dyn_cast<CallInst>(User);
- if (!callInstIsMemcpy(CI) || !destArrayCanBeWidened(CI))
- continue;
-
- if (CI->getArgOperand(1) != SourceVar)
- continue;
-
- widenDestArray(CI, NumBytesToPad, NumBytesToCopy, SourceDataArray);
-
- CI->setArgOperand(2, ConstantInt::get(BytesToCopyOp->getType(),
- NumBytesToCopy + NumBytesToPad));
- }
- SourceVar->replaceAllUsesWith(NewSourceGV);
-
- NumGlobalArraysPadded++;
- return true;
-}
-
-static bool tryWidenGlobalArraysUsedByMemcpy(
- GlobalVariable *GV,
- function_ref<TargetTransformInfo &(Function &)> GetTTI) {
-
- if (!GV->hasInitializer() || !GV->isConstant() || !GV->hasLocalLinkage() ||
- !GV->hasGlobalUnnamedAddr())
- return false;
-
- for (auto *User : GV->users()) {
- CallInst *CI = dyn_cast<CallInst>(User);
- if (!callInstIsMemcpy(CI) || !destArrayCanBeWidened(CI))
- continue;
-
- auto *BytesToCopyOp = dyn_cast<ConstantInt>(CI->getArgOperand(2));
- if (!BytesToCopyOp)
- continue;
-
- ConstantDataArray *SourceDataArray =
- dyn_cast<ConstantDataArray>(GV->getInitializer());
- if (!SourceDataArray)
- continue;
-
- unsigned NumBytesToCopy = BytesToCopyOp->getZExtValue();
-
- auto *Alloca = cast<AllocaInst>(CI->getArgOperand(0));
- uint64_t DZSize = Alloca->getAllocatedType()->getArrayNumElements();
- uint64_t SZSize = SourceDataArray->getType()->getNumElements();
- unsigned ElementByteWidth = SourceDataArray->getElementByteSize();
- // Calculate the number of elements to copy while avoiding floored
- // division of integers returning wrong values i.e. copying one byte
- // from an array of i16 would yield 0 elements to copy as supposed to 1.
- unsigned NumElementsToCopy = divideCeil(NumBytesToCopy, ElementByteWidth);
-
- // For safety purposes lets add a constraint and only pad when
- // NumElementsToCopy == destination array size ==
- // source which is a constant
- if (NumElementsToCopy != DZSize || DZSize != SZSize)
- continue;
-
- unsigned NumBytesToPad =
- GetTTI(*CI->getFunction())
- .getNumBytesToPadGlobalArray(NumBytesToCopy,
- SourceDataArray->getType());
- if (NumBytesToPad) {
- return tryWidenGlobalArrayAndDests(GV, NumBytesToPad, NumBytesToCopy,
- BytesToCopyOp, SourceDataArray);
- }
- }
- return false;
-}
-
static bool
OptimizeGlobalVars(Module &M,
function_ref<TargetTransformInfo &(Function &)> GetTTI,
@@ -2225,10 +2064,6 @@ OptimizeGlobalVars(Module &M,
continue;
}
- // For global variable arrays called in a memcpy
- // we try to pad to nearest valid alignment boundary
- Changed |= tryWidenGlobalArraysUsedByMemcpy(&GV, GetTTI);
-
Changed |= processGlobal(GV, GetTTI, GetTLI, LookupDomTree);
}
return Changed;
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-dest-non-array.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-dest-non-array.ll
deleted file mode 100644
index ab04e0a5bc697..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-dest-non-array.ll
+++ /dev/null
@@ -1,39 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.i8 = private unnamed_addr constant [3 x i8] [i8 1, i8 2, i8 3] , align 1
-
-define void @memcpy_struct() {
-; CHECK-LABEL: define void @memcpy_struct() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca { i8, i8, i8 }, align 1
-; CHECK-NEXT: [[CALL1:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca {i8, i8, i8}, align 1
- %call1 = call i32 @bar(ptr nonnull %something)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- ret void
-}
-
-
-@.i8_multi = private unnamed_addr constant [2 x [3 x i8]] [[3 x i8] [i8 1, i8 2, i8 3], [3 x i8] [i8 4, i8 5, i8 6]] , align 1
-
-define void @memcpy_array_multidimensional() {
-; CHECK-LABEL: define void @memcpy_array_multidimensional() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [2 x [3 x i8]], align 1
-; CHECK-NEXT: [[CALL1:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(3) @.i8_multi, i32 3, i1 false)
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [2 x [3 x i8]], align 1
- %call1 = call i32 @bar(ptr nonnull %something)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something, ptr noundef nonnull align 1 dereferenceable(3) @.i8_multi, i32 3, i1 false)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-global-dest.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-global-dest.ll
deleted file mode 100644
index f435ffdeed2c8..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-global-dest.ll
+++ /dev/null
@@ -1,28 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-; CHECK: [3 x i8]
-@other = private unnamed_addr global [3 x i8] [i8 1, i8 2, i8 3] , align 1
-; CHECK: [4 x i8]
-@.i8 = private unnamed_addr constant [3 x i8] [i8 1, i8 2, i8 3] , align 1
-
-define void @memcpy_multiple() {
-; CHECK-LABEL: define void @memcpy_multiple() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: [[CALL3:%.*]] = call i32 @bar(ptr nonnull @other)
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) @other, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 4, i1 false)
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [3 x i8], align 1
- %call1 = call i32 @bar(ptr nonnull %something)
- %call2 = call i32 @bar(ptr nonnull @other)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) @other, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-byte-array.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-byte-array.ll
deleted file mode 100644
index c7ca7271fd3d2..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-byte-array.ll
+++ /dev/null
@@ -1,22 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.i16 = private unnamed_addr constant [5 x i16] [i16 1, i16 2, i16 3, i16 4, i16 5] , align 1
-
-define void @memcpy_i16_array() {
-; CHECK-LABEL: define void @memcpy_i16_array() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING1:%.*]] = alloca [6 x i16], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(10) [[SOMETHING1]], ptr noundef nonnull align 1 dereferenceable(10) @.i16, i32 12, i1 false)
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING1]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [5 x i16], align 1
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(10) %something, ptr noundef nonnull align 1 dereferenceable(10) @.i16, i32 10, i1 false)
- %call2 = call i32 @bar(ptr nonnull %something)
- ret void
-}
-
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-const-global.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-const-global.ll
deleted file mode 100644
index 3d9c42fe1f3dd..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-const-global.ll
+++ /dev/null
@@ -1,21 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.str = unnamed_addr global [3 x i8] c"12\00", align 1
-
-define void @foo() {
-; CHECK-LABEL: define void @foo() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [3 x i8], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(3) @.str, i32 3, i1 false)
-; CHECK-NEXT: [[CALL1:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [3 x i8], align 1
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something, ptr noundef nonnull align 1 dereferenceable(3) @.str, i32 3, i1 false)
- %call1 = call i32 @bar(ptr nonnull %something)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-string-multi-use.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-string-multi-use.ll
deleted file mode 100644
index e37925a78d2c3..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-string-multi-use.ll
+++ /dev/null
@@ -1,33 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.i8 = private unnamed_addr constant [3 x i8] [i8 1, i8 2, i8 3] , align 1
-
-define void @memcpy_multiple() {
-; CHECK-LABEL: define void @memcpy_multiple() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING2:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT: [[SOMETHING1:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT: [[SOMETHING3:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING2]], ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 4, i1 false)
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING1]], ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 4, i1 false)
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) [[SOMETHING3]], ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 4, i1 false)
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING2]])
-; CHECK-NEXT: [[CALL3:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING1]])
-; CHECK-NEXT: [[CALL4:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING3]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [3 x i8], align 1
- %something1 = alloca [3 x i8], align 1
- %something2 = alloca [3 x i8], align 1
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something1, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(3) %something2, ptr noundef nonnull align 1 dereferenceable(3) @.i8, i32 3, i1 false)
- %call2 = call i32 @bar(ptr nonnull %something)
- %call3 = call i32 @bar(ptr nonnull %something1)
- %call4 = call i32 @bar(ptr nonnull %something2)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-1.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-1.ll
deleted file mode 100644
index 8ea9e2804370e..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-1.ll
+++ /dev/null
@@ -1,21 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.str = private unnamed_addr constant [10 x i8] c"123456789\00", align 1
-
-define void @foo() {
-; CHECK-LABEL: define void @foo() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [12 x i8], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(10) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(10) @.str, i32 12, i1 false)
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [10 x i8], align 1
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(10) %something, ptr noundef nonnull align 1 dereferenceable(10) @.str, i32 10, i1 false)
- %call2 = call i32 @bar(ptr nonnull %something)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-2.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-2.ll
deleted file mode 100644
index ad3620b14ea23..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-2.ll
+++ /dev/null
@@ -1,21 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-
-@.str = private unnamed_addr constant [62 x i8] c"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\00", align 1
-
-define void @foo() {
-; CHECK-LABEL: define void @foo() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [64 x i8], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(62) [[SOMETHING]], ptr noundef nonnull align 1 dereferenceable(62) @.str, i32 64, i1 false)
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [62 x i8], align 1
- call void @llvm.memcpy.p0.p0.i32(ptr noundef nonnull align 1 dereferenceable(62) %something, ptr noundef nonnull align 1 dereferenceable(62) @.str, i32 62, i1 false)
- %call2 = call i32 @bar(ptr nonnull %something)
- ret void
-}
-
-declare i32 @bar(...)
diff --git a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-lengths-dont-match.ll b/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-lengths-dont-match.ll
deleted file mode 100644
index b8e02c3f996da..0000000000000
--- a/llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-lengths-dont-match.ll
+++ /dev/null
@@ -1,26 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -mtriple=arm-none-eabi -passes=globalopt -S | FileCheck %s
-; CHECK: [17 x i8]
-@.str = private unnamed_addr constant [17 x i8] c"aaaaaaaaaaaaaaaa\00", align 1
-
-; Function Attrs: nounwind
-define void @foo() {
-; CHECK-LABEL: define void @foo() local_unnamed_addr {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[SOMETHING:%.*]] = alloca [20 x i8], align 1
-; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 20, ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr nonnull align 1 [[SOMETHING]], ptr align 1 @.str, i32 17, i1 false)
-; CHECK-NEXT: [[CALL2:%.*]] = call i32 @bar(ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 20, ptr nonnull [[SOMETHING]])
-; CHECK-NEXT: ret void
-;
-entry:
- %something = alloca [20 x i8], align 1
- call v...
[truncated]
|
tgymnich
approved these changes
Jun 18, 2025
fhahn
reviewed
Jun 18, 2025
I could do that, but I think it would be easier to review a reland if it contained all the tests (otherwise it's non-obvious that all negative tests are present). |
fhahn
approved these changes
Jun 21, 2025
There was a problem hiding this comment.
LGTM, thanks
Does any of the removed tests show a correctness issue? If so, would be good to keep them or otherwise add a new test?
I could do that, but I think it would be easier to review a reland if it contained all the tests (otherwise it's non-obvious that all negative tests are present).
SGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially reverts e37d736.
The transform has a number of correctness and code quality issues, and will benefit from a from-scratch re-review more than incremental fixes.
The correctness issues are hinted at in #144641, but I think it needs a larger rework to stop working on ArrayTypes and the implementation could use some other improvements (like callInstIsMemcpy should just be
dyn_cast<MemCpyInst>). I can comment in more detail on a resubmission of the patch.